Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

osal Integration candidate: 2021-01-12 #750

Merged
merged 34 commits into from
Jan 13, 2021
Merged

Conversation

astrogeco
Copy link
Contributor

@astrogeco astrogeco commented Jan 11, 2021

Describe the contribution

Fix #702, use iterators instead of for loops
Fix #645, implement separate lock/unlock key
Fix #703, implement exclusive/reserved lock
Fix #642, make OS_TaskDelete sync
Fix #703, unit test
Fix #697, use POSIX dir implementation on V
Fix #580, improve FS_BASED mounts on
Fix #708, chmod error h
Fix #471, order of operations for delete all
Fix #445, add pointer parameter checks
Fix #573, add OS_FileSysSta
Fix #544, add pointer check
Fix #606, Resolve cast-align error in VxWorks OS_TaskGet
Fix #429, update OSAL code to use time ac
Fix #429, check time conversions in coverage test
Fix #644, Remove alignment macros
Fix #429, add "assemble" routines for milli/mi
Fix #732 change uint32 to size_t
Fix #755, resolve subtasks not ending on time

Testing performed
See https://github.com/nasa/cFS/pull/174/checks

Expected behavior changes

PR #704

PR #710

  • The chmod test is now skipped on VxWorks rather than failing. The OS_FileChmod_Impl() function now returns OS_ERR_NOT_IMPLEMENTED when run on a file system that does not have permissions, which in turn causes the unit test to be skipped rather than fail.
  • Corrects a file handle leak.

PR #716

  • Add parameter check to OS_SocketSendTo and adjust coverage test to validate.

PR #717

  • Replace OS_fsBytesFree and OS_fsBlocksFree with OS_FileSysStatVolume. This new API for getting stats on file system. Uses existing OS_FileSysStatVolume_Impl call and exposes it in the public API.

PR #711

  • When cleaning up for shutdown, delete resources that have a task/thread first, followed by other resource types. This helps avoid possible dependencies as running threads might be using the other resources. No detectable external impact; internally, the tasks are deleted first during shutdown, which only has an impact if/when tasks are actively using other OSAL resources.

PR #709

  • The mount/unmount VxWorks implementation was not adequately checking for and handling the FS_BASED pass -through mapping type - which should be mostly a no-op. Create a mount point directory if it does not already exist when using this mapping type for consistency with POSIX.
  • Adds a documentation note to OS_FileSysAddFixedMap(): The virtual mount point cannot be empty - so OS_FileSysAddFixedMap(.., "/", "/") does not work but OS_FileSysAddFixedMap(.., "/", "/root") does work and allows one to open files in the root as "/root/" from OSAL applications. Mount-point directories do not need to previously exist when using OS_FileSysAddFixedMap

PR #720

  • store taskTCB return in a void *, then cast to OS_impl_task_internal_record_t * to avoid a strict alignment compiler error

PR #734

  • Removes the non-portable OS_PACK and OS_ALIGNED macros.

PR #706

  • Uses the POSIX dir implementation on VxWorks 6.9. The only incompatibility is the prototype for mkdir()which is missing the second argument; this is worked around with a compatibility macro for VxWorks 6.x builds.
  • Translate and convert the VxWorks coverage test cases to the portable dir implementation, which benefits VxWorks7, RTEMS, and POSIX.

PR #733

  • Fixes prototypes so they run on RTEMS by replacing uint32 with size_t

PR #715

  • Adds OS_CHECK_POINTER macros to OS_ConvertToArrayIndex and OS_TimeBaseGetFreeRun so they can handle NULL pointers and return the correct error.

PR #723

  • Adds access functions to convert/extract different units from an OS_time_t value - so that other code in CFE/PSP/Apps can be updated to use the access functions and thereby not break when the internal time definition changes. Replaces the int32 with OS_time_t in the "stat" structure used by the file module. Updates the pointer argument to OS_SetLocalTime() to be const. Prototype change of OS_SetLocalTime() should be backward compatible.

Additional context
Part of nasa/cFS#174

Third party code
None

Contributor Info - All information REQUIRED for consideration of pull request
@skliper
@jphickey
@zanzaben

jphickey and others added 24 commits December 21, 2020 13:48
Convert remaining operations using for loops to use iterators.
This ensures locking is done consistently and correctly.
Implement an "unlock key" - based on task ID - which can be part of the local
token, rather than relying on the task ID being the same between the lock
and unlock ops.  Notably, the task ID can change, in particular if the task
is exiting.

Also Fixes #701, other general cleanup

Implement all global lock/unlock error checking in shared layer, not in
impl layer, for consistency.  Remove redundant checks.

Make all functions return void (should never fail) and in the
unlikely event that something does fail then report the error,
but no other recourse possible.
Change the EXCLUSIVE lock type such that it sets the ID in the global
table to RESERVED and unlocks the global before returning to the caller.

This allows the potentially long-running operation to complete and
not block other operations from happening in other tasks.

Use the EXCLUSIVE lock for all create/delete ops as well as for
bind and connect socket ops.

Also implement a new "RESERVED" lock to handle a special case in the
vxworks timebase implementation where the impl layer needs to acquire
a token for an object as it is being created.  This case is special
because it needs to happen during OS_TimeBaseCreate, and cannot be
completed after the fact like normal tasks, because it is a factor
in determining the success/fail status of the overall operation.
In the POSIX implementation, OS_TaskDelete was implemented in a
deferred manner - the API call was a request, and the actual
deletion occured sometime thereafter.  This is a problem if the
task is running code within a dynamically loaded module, and the
intent is to delete the task so the module can be unloaded.  In
this case the app needs to be certain that the task has actually
been deleted before unloading can be done safely.

To do this requires use of pthread_join() on POSIX which confirms
that the task has exited.  However, this is a (potentially) blocking
call, so to do this requires rework of the EXCLUSIVE lock mode
such that the OSAL lock is _not_ held during the join operation.
Update unit tests for idmap functions, add test cases where coverage
was incomplete.  All OS_ObjectId* function coverage is back at 100%.
Instead of maintaining a one-off implementation for VxWorks 6,
use the POSIX implementation for this module.  The only
incompatibility is the prototype for mkdir() which is missing
the second argument.  This can be worked around with a simple
compatibility macro that is only enabled for VxWorks 6.x builds.
The mount/unmount implementation was not really checking
for and handling this mapping type.

To be consistent with POSIX it should also create a directory
if it does not already exist.
Better error translations in the OS_FileChmod_Impl() function.
Also corrects a file handle leak.

This makes it return OS_ERR_NOT_IMPLEMENTED when run on a file
system that does not have permissions, which in turn causes the
unit test to be skipped rather than fail.
When cleaning up for shutdown, delete resources that have
a task/thread first, followed by other resource types.

This helps avoid possible dependencies as running threads
might be using the other resources.
Add OS_CHECK_POINTER macros to OS_ConvertToArrayIndex and
OS_TimeBaseGetFreeRun.
Add OS_FileSysStatVolume as replacement for OS_fsBytesFree and
OS_fsBlocksFree.  Update unit tests and stubs for the new API
call.

Does not (yet) deprecate the existing functions, as references
still need to be updated elsewhere in apps.
Add parameter check to OS_SocketSendTo and adjust coverage test
to validate.
Do not access members of OS_time_t directly, instead
use conversion/accessor inline functions to get the
desired value.

Update the "stat" structure output by OS_stat to use
the OS_time_t struct instead of int32, and update
the OS_stat implemention to transfer the full resolution
if it supports it (POSIX.1-2008 or newer).
Add test cases to coverage test to validate all basic
OS_time_t access operations and conversions.
Add OS_TimeAssembleFromMilliseconds and OS_TimeAssembleFromMicroseconds
for a complete set of conversion routines in both directions.
Fix #642, 645, 701, 702, 703 - OSAL global table management
Fix #471, order of operations for delete all
@astrogeco
Copy link
Contributor Author

astrogeco commented Jan 12, 2021

@jphickey Got some doxygen errors with the last merge

/home/runner/work/cFS/cFS/osal/src/os/inc/osapi-filesys.h:71: warning: Unsupported xml/html tag <virt_mount_point> found
/home/runner/work/cFS/cFS/osal/src/os/inc/osapi-filesys.h:71: warning: Unsupported xml/html tag <relative_path> found
/home/runner/work/cFS/cFS/osal/src/os/inc/osapi-filesys.h:71: warning: Unsupported xml/html tag <file> found
/home/runner/work/cFS/cFS/osal/src/os/inc/osapi-filesys.h:72: warning: Unsupported xml/html tag <virt_mount_point> found
/home/runner/work/cFS/cFS/osal/src/os/inc/osapi-filesys.h:72: warning: Unsupported xml/html tag <file> found

Added Doxygen escape character "\" to "<>" symbols in comments
@astrogeco astrogeco force-pushed the integration-candidate branch from 6b43e19 to f9dd6b3 Compare January 13, 2021 04:07
@astrogeco
Copy link
Contributor Author

@jphickey Got some doxygen errors with the last merge

/home/runner/work/cFS/cFS/osal/src/os/inc/osapi-filesys.h:71: warning: Unsupported xml/html tag <virt_mount_point> found
/home/runner/work/cFS/cFS/osal/src/os/inc/osapi-filesys.h:71: warning: Unsupported xml/html tag <relative_path> found
/home/runner/work/cFS/cFS/osal/src/os/inc/osapi-filesys.h:71: warning: Unsupported xml/html tag <file> found
/home/runner/work/cFS/cFS/osal/src/os/inc/osapi-filesys.h:72: warning: Unsupported xml/html tag <virt_mount_point> found
/home/runner/work/cFS/cFS/osal/src/os/inc/osapi-filesys.h:72: warning: Unsupported xml/html tag <file> found

Fixed, see f9dd6b3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment